Fix word-break in tables on most browsers#1349
Merged
cchaos merged 9 commits intoelastic:masterfrom Dec 7, 2018
Merged
Conversation
Contributor
Author
|
@cjcenizal Would you mind taking a peak at this PR since you were the one who originally added the word-wrap stuff? @peteharverson Can you check if this fixes your issue? IE11 will still behave poorly because it just doesn't support word-break. |
This reverts commit f457b73.
added 3 commits
December 6, 2018 13:23
and using on table cell contents
Contributor
Author
|
@cjcenizal & @peteharverson Based on more investigating, I've updated the description of the PR and changed the implementation of the "fix". cc @snide |
cjcenizal
approved these changes
Dec 6, 2018
Contributor
cjcenizal
left a comment
There was a problem hiding this comment.
Looks great! Nice analysis. Thanks for digging in and explaining everything so clearly. I just had a couple minor suggestions.
Contributor
Author
|
@cjcenizal Is this documentation more helpful? |
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

The problem came about that browser's were breaking on words unnecessarily (in Firefox and IE11). The problem is that neither Firefox nor IE support
word-break: break-wordand so there is a fallback toword-break: break-allto ensure that long words don't expand past container.What it looked like before:
So this PR changes from using the
word-break: break-wordapproach to usingoverflow-wrap: break-wordwhich is supported by all browsers except for IE11 and behaves just likeword-break.This is what it looks like now:
Helpers
I've added both a mixin (
euiTextOverflowWrap()) and CSS utility (.eui-textOverflowWrap) to handle the ease of usingoverflow-wrapand the IE fallback.Caveat
There is one big caveat to using
overflow-wrap. It does not work on items that havedisplay: flex. Wah wah...Where this becomes an issue is when passing
textOnly: falsetoEuiTableRowCellwhich removes the containing.euiTableCellContent__textand places the children as a direct descendent ofeuiTableCellContentwhich, guess what?, isdisplay: flex. 😢So it has to use the
word-breakimplementation, meaning it will also use thebreak-allfallback for Firefox.This usually isn't an issue because the default prop is
textOnly: true. However,EuiBasicTablewill turn that prop to false if you use therenderfunction.What it looks like when
textOnly: false:Firefox will still break on
allbecause the containing element is a flex element.I've updated the documentation to try to spell out how to fix this, usually by keeping/adding
textOnly: true. But another way to combat this is to just wrap the text node with the.eui-textOverflowWraputility class.IE11
Blech. Unfortunately, IE11 has no support any type of
break-wordso it will continue to use thebreak-allfall back.Checklist
[ ] This was checked in mobile[ ] This was checked in dark mode[ ] This was checked for breaking changes and labeled appropriately[ ] Jest tests were updated or added to match the most common scenarios[ ] This was checked against keyboard-only and screenreader scenarios[ ] This required updates to Framer X components